fix(security): per-buyer idempotency, write scope, atomic async cache write - #482
Open
abojeEdwin wants to merge 3 commits into
Open
fix(security): per-buyer idempotency, write scope, atomic async cache write#482abojeEdwin wants to merge 3 commits into
abojeEdwin wants to merge 3 commits into
Conversation
Accept an optional Idempotency-Key header on POST /api/talos/:id/service and persist the request/result association so safe retries after browser, network, or Stellar submission timeouts create at most one job and one payment effect. Idempotency contract ──────────────────── - No header → processed normally (backward compatible) - New key → job created, 201 response cached, X-Idempotent-Replayed: false - Same key + same payload → original cached 201 returned, X-Idempotent-Replayed: true - Same key + different payload → 409 Conflict - Concurrent requests with same key → 409 'already being processed' - Different buyer with same key → allowed (scoped per buyer) Key is bound to the authenticated buyer, target service, and request payload so it cannot be replayed across users or purchases. Changes: - Add Idempotency-Key header reading and 128-byte validation to service route - Add idempotency check before payment verification/settlement - Cache response body (idempotencyResponse) in same DB transaction as job insert - Handle 23505 unique constraint violations for concurrent race conditions - Add composite unique index (talosId, requesterTalosId, idempotencyKey) migration - Update OpenAPI spec with Idempotency-Key parameter and idempotency contract docs - Add 10 tests covering: new key, equivalent retry, payload conflict, in-flight, concurrent race (Promise.all), paymentSig replay, key length, and per-buyer scoping
…y-keys-purchase-requests
…ache write Address review blockers for service purchase idempotency: - Drop the provider-scoped (talosId, idempotencyKey) unique index and replace it with the composite (talosId, requesterTalosId, idempotencyKey) index so two buyers can reuse the same key on one service. Renumbered migration to 0018 to avoid clashing with main's 0017_add_scoped_api_keys. - POST /api/talos/:id/service is a mutating purchase; require the commerce:write scope instead of commerce:read. - Make the async job insert + idempotencyResponse cache write atomic via a single transaction so a failed cache write rolls back the insert (no orphaned row, no permanent 409 on retry). Adds regression tests for the write scope, the index boundary, and the atomic async cache write (including recovery on retry).
|
@abojeEdwin is attempting to deploy a commit to the Cankat's projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Hi @enliven17, apologies for the delay |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #396
Resubmission of #407, addressing all review blockers:
Review blockers fixed
Conflicting unique index removed. The migration now drops the old
provider-scoped
(talosId, idempotencyKey)unique index and replaces itwith the composite
(talosId, requesterTalosId, idempotencyKey)index, sotwo different buyers can reuse the same key on one service without
colliding. Schema, generated Drizzle schema, and the 0015 meta snapshot are
kept in sync. Renumbered to
0018to avoid clashing with main's0017_add_scoped_api_keys.Write scope.
POST /api/talos/:id/serviceis a mutating purchase andnow authenticates with
commerce:writeinstead ofcommerce:read.Atomic async cache write. The async path wraps the job insert and the
idempotencyResponsecache write in a single transaction viawithTransactionRetry. A failed cache write now rolls back the insertinstead of leaving an orphaned row that would turn every retry into a
permanent 409.
Feature summary
Accept an optional
Idempotency-Keyheader onPOST /api/talos/:id/serviceso safe retries after browser, network, or Stellar submission timeouts create
at most one job and one payment effect. The key is bound to the authenticated
buyer, target service, and request payload.
X-Idempotent-Replayed: falseTests
Idempotency suite expanded from 10 to 14 tests, adding regression coverage for:
commerce:writescope requirement on the purchase routeAlso verified: no new test failures and no new type errors vs. the merged
main baseline (the pre-existing failures in unrelated areas are unchanged).
Test Plan
pnpm dlx vitest run service-purchase-idempotency openapi-snapshotpassestsc --noEmitshows zero new type errors vs. merged baselineeslinton changed files: 0 errors